-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Simplify group autorisation's synchronisation from LDAP #772
base: master
Are you sure you want to change the base?
Conversation
pour ma part je n'utilise pas, mais ca me semble simplifier les choses :) |
J'ai testé et avec quelques corrections ça fonctionne. Il faudra mettre une release note pour ceux qui l'utilisaient déjà :
ou
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L'utilisation d'une classe de configuration pour ajouter les beans nécessaires fait très "spring-boot" dans l'approche, et ca a l'air de "clasher" avec le fait que cadastrapp utilise plutôt une définition des beans dans un fichier XML, mais ca ne me choque pas outre mesure.
J'aime bien le fait que cela reste désactivable, au cas où on voudrait quand même avoir son propre cronjob externe, cela laisse la liberté à l'utilisateur de le faire.
Je ne maîtrise pas très bien la config via des fichiers XML, et pour ma part je trouve ça relativement illisible, surement par manque d'habitude. Lors de mes tests l'utilisation des deux en parallèle ne semble pas poser de soucis. |
Cette PR permet d'utiliser Spring au lieu de Postgres/Multicorn pour la sync avec le LDAP, ce qui simplifie grandement le paramétrage.
Je n'ai pas encore eu le temps de tester, mais en attendant je suis preneur de vos retours/avis.